Skip to content

fix(search-replace): don't auto-navigate when content edits invalidate the active match#4819

Merged
waleedlatif1 merged 7 commits into
stagingfrom
fix/sar
May 31, 2026
Merged

fix(search-replace): don't auto-navigate when content edits invalidate the active match#4819
waleedlatif1 merged 7 commits into
stagingfrom
fix/sar

Conversation

@waleedlatif1
Copy link
Copy Markdown
Collaborator

Summary

  • When manually editing a block's content caused the active search match to disappear, the panel was immediately jumping to a different block — stealing focus mid-edit
  • Fixed by distinguishing between matches disappearing due to a query change (should auto-navigate) vs content changes (should not)
  • On content-driven disappearance: deselects the match and shows "0 of N" so the user can navigate explicitly with the arrow keys
  • Replace button correctly advances to the next match (VS Code behavior) rather than jumping to index 0
  • Fixed a secondary bug where pressing ↓ from a deselected state skipped the first match entirely

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel Bot commented May 31, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped May 31, 2026 1:50am

Request Review

@cursor
Copy link
Copy Markdown

cursor Bot commented May 31, 2026

PR Summary

Low Risk
UI-only navigation and selection logic in the workflow search panel; no auth, data, or API changes.

Overview
Fixes workflow search and replace so the panel no longer jumps to another match when edits make the current hit disappear—only query changes or opening the panel auto-select the first result.

When the active match drops out for other reasons, selection is cleared and the counter shows 0 of N until the user uses arrows. Replace records the current index so the next match is selected at that position (VS Code–style) instead of resetting to the first hit; failed applies clear that intent. Arrow navigation from a deselected state no longer skips the first or last match.

Minor cleanup: panel close clears the editor search target; some handlers drop unnecessary useCallback/useMemo.

Reviewed by Cursor Bugbot for commit 680fa11. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented May 31, 2026

Greptile Summary

This PR fixes a focus-stealing bug in the search-replace panel: when a user manually edits a block's content and causes the active match to disappear, the panel now deselects silently ("0 of N") instead of jumping to a different block mid-edit. It also aligns Replace-button navigation with VS Code behavior and fixes ↓ from a deselected state skipping the first match.

  • Adds prevQueryRef, prevIsOpenRef, and afterReplaceIndexRef to distinguish query-driven vs. content-driven match disappearances, navigating only when appropriate (queryChanged || justOpened || replaceIndex !== null).
  • Introduces a committed flag in handleApply with finally-block cleanup so stale afterReplaceIndexRef values are cleared on any failure path (conflicts, empty batch, !applied), and moves the ref write past the top-level guard to prevent stale refs from double-clicks during an in-flight replace.
  • Fixes the deselected-state navigation in handleMoveActiveMatch so ↓ goes to index 0 and ↑ goes to the last match, rather than both starting at index 1.

Confidence Score: 5/5

Safe to merge — the fix is well-scoped, the stale-ref edge cases raised in prior review rounds are all addressed, and the navigation logic holds up across the key scenarios.

All three stale-afterReplaceIndexRef paths discussed in prior threads are now closed: the committed flag + finally clears the ref on every failure path inside the try block, the ref write was moved past the top-level guard so a double-click during an in-flight apply cannot leave it set, and the zero-matches branch also clears it. The justOpened || queryChanged vs. content-edit distinction in the navigation effect is correct, and the deselected-state arrow-key fix matches the described VS Code behavior.

No files require special attention.

Important Files Changed

Filename Overview
apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/search-replace/workflow-search-replace.tsx Core fix: adds ref-based change tracking to distinguish query vs. content-driven match disappearances, with correct stale-ref cleanup via committed flag in handleApply; navigation logic and deselected-state arrow key behavior are correct.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[hydratedMatches changes] --> B{isOpen?}
    B -- No --> C[prevIsOpenRef = false\nsetActiveSearchTarget null\nreturn]
    B -- Yes --> D[Compute justOpened\nCompute queryChanged\nUpdate prevQueryRef / prevIsOpenRef]
    D --> E{hydratedMatches\nlength === 0?}
    E -- Yes --> F[Clear afterReplaceIndexRef\nClear activeMatchId\nsetActiveSearchTarget null]
    E -- No --> G{activeMatchId still\nin hydratedMatches?}
    G -- Yes --> H[setActiveSearchTarget\nwith current match]
    G -- No --> I[Read & clear\nafterReplaceIndexRef]
    I --> J{queryChanged\nor justOpened?}
    J -- Yes --> K[Navigate to\nhydratedMatches 0]
    J -- No --> L{replaceIndex\n!== null?}
    L -- Yes --> M[Navigate to\nMin replaceIndex, last match index]
    L -- No --> N[Deselect: setActiveMatchId null\nsetActiveSearchTarget null\nShow 0 of N]
Loading

Reviews (5): Last reviewed commit: "fix(search-replace): revert !activeMatch..." | Re-trigger Greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3aeb9dc. Configure here.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 3475fe4. Configure here.

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
Copy link
Copy Markdown
Collaborator Author

@cursor review

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit 680fa11. Configure here.

@waleedlatif1 waleedlatif1 merged commit 1ae1afb into staging May 31, 2026
14 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/sar branch May 31, 2026 01:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant